Skip to content

Change Firestore credentials to use shared_ptr#1827

Merged
a-maurice merged 7 commits intomainfrom
am-firestore_fix
Feb 23, 2026
Merged

Change Firestore credentials to use shared_ptr#1827
a-maurice merged 7 commits intomainfrom
am-firestore_fix

Conversation

@a-maurice
Copy link
Contributor

@a-maurice a-maurice commented Feb 18, 2026

Description

Provide details of the change, and generalize the change in the PR title above.

Change Firestore's Auth and App Check credentials to use shared_ptr's instead of unique_ptr. This is because the iOS repo changed to use shared_ptrs recently, and the mismatch seems to be causing runtime errors. See firebase/firebase-ios-sdk#15558

Fixes firebase/firebase-unity-sdk#1383

Testing

Describe how you've tested these changes. Link any manually triggered Integration tests or CPP binary SDK Packaging Github Action workflows, if applicable.

Running tests locally

Type of Change

Place an x the applicable box:

  • Bug fix. Add the issue # below if applicable.
  • New feature. A non-breaking change which adds functionality.
  • Other, such as a build process or documentation change.

Notes

  • Bug fixes and feature changes require an update to the Release Notes section of release_build_files/readme.md.
  • Read the contribution guidelines CONTRIBUTING.md.
  • Changes to the public API require an internal API review. If you'd like to help us make Firebase APIs better, please propose your change in a feature request so that we can discuss it together.

@gemini-code-assist
Copy link
Contributor

gemini-code-assist bot commented Feb 18, 2026

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@a-maurice a-maurice added the tests-requested: quick Trigger a quick set of integration tests. label Feb 18, 2026
@github-actions github-actions bot added tests: in-progress This PR's integration tests are in progress. and removed tests-requested: quick Trigger a quick set of integration tests. labels Feb 18, 2026
@github-actions
Copy link

github-actions bot commented Feb 18, 2026 •
edited
Loading

✅  Integration test succeeded!

Requested by @a-maurice on commit c7fd131
Last updated: Mon Feb 23 17:04 PST 2026
View integration test log & download artifacts

@github-actions github-actions bot added the tests: failed This PR's integration tests failed. label Feb 18, 2026
@firebase-workflow-trigger firebase-workflow-trigger bot removed the tests: in-progress This PR's integration tests are in progress. label Feb 18, 2026
@a-maurice a-maurice added tests-requested: quick Trigger a quick set of integration tests. and removed tests: failed This PR's integration tests failed. labels Feb 18, 2026
@github-actions github-actions bot added tests: in-progress This PR's integration tests are in progress. tests: succeeded This PR's integration tests succeeded. and removed tests-requested: quick Trigger a quick set of integration tests. labels Feb 18, 2026
@firebase-workflow-trigger firebase-workflow-trigger bot removed the tests: in-progress This PR's integration tests are in progress. label Feb 18, 2026
@a-maurice
Copy link
Contributor Author

a-maurice commented Feb 23, 2026

/gemini review

gemini-code-assist[bot]
gemini-code-assist bot reviewed Feb 23, 2026
View reviewed changes
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request correctly replaces std::unique_ptr with std::shared_ptr for Firestore's authentication and App Check credentials providers. This change is necessary to align with the iOS SDK and resolve runtime errors. The modifications are applied consistently across all relevant files, including function signatures, implementations, and test utilities. The release notes have also been updated to reflect this fix. I have one minor suggestion regarding include order to better align with style guidelines.

firestore/src/main/create_app_check_credentials_provider_ios.mm

#include "firestore/src/main/create_credentials_provider.h"

#include <memory>
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

To adhere to the Google C++ Style Guide for include order, C++ standard library headers like <memory> should be placed before project-specific headers. Please move this include before #include "firestore/src/main/create_credentials_provider.h".

cherylEnkidu
cherylEnkidu approved these changes Feb 23, 2026
View reviewed changes
Copy link
Contributor

@cherylEnkidu cherylEnkidu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the fix!

@a-maurice a-maurice merged commit c7fd131 into main Feb 23, 2026
29 checks passed
@a-maurice a-maurice deleted the am-firestore_fix branch February 23, 2026 23:12
@github-actions github-actions bot added tests: in-progress This PR's integration tests are in progress. tests: succeeded This PR's integration tests succeeded. and removed tests: succeeded This PR's integration tests succeeded. labels Feb 23, 2026
@firebase-workflow-trigger firebase-workflow-trigger bot removed the tests: in-progress This PR's integration tests are in progress. label Feb 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cherylEnkidu cherylEnkidu cherylEnkidu approved these changes

+1 more reviewer

@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

tests: succeeded This PR's integration tests succeeded.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug] Firestore (bad_weak_ptr) crash on iOS 15.5 for Firestore SDK 13.6.0+

2 participants

@a-maurice @cherylEnkidu